-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Runtime-async Exception.ToString() #122722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add new JIT-EE API to report back debug information about the generated state machine and continuations - Refactor debug info storage on VM side to be more easily extensible. The new format has either a thin or fat header. The fat header is used when we have either uninstrumented bounds, patchpoint info, rich debug info or async debug info, and stores the blob sizes of all of those components in addition to the bounds and vars. - Add new async debug information to the storage on the VM side - Set get target method desc for async resumption stubs, to be used for mapping from continuations back to the async IL function that it will resume.
…mental APIs in async tests
src/coreclr/vm/excep.cpp
Outdated
| _ASSERTE(IsException(pMT)); | ||
|
|
||
| PTR_ThreadExceptionState pCurTES = pThread->GetExceptionState(); | ||
| // Check if the flag indicating foreign exception raise has been setup or not, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janvorli Could you please review these changes? In particular, the changes around IsRaisingForeignException flag for runtime async frames. Do we need to worry about it at all for runtime async frames?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not believe so. This flag is only set during ExceptionDispatchInfo.Throw(), so the only place it would matter is if Edi.Throw() is immediately above a runtime-async frame in a stacktrace. However, above a runtime-async frame we will either have another runtime-async frame, or DispatchContinuations, the only call site of this method.
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| } | ||
| } | ||
|
|
||
| #if !(NATIVEAOT && !WINDOWS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt that this is going to work. The build of libraries partition is runtime flavor agnostic. The only place under libraries where you can use NATIVEAOT ifdef is CoreLib - that's because the CoreLib is not actually built from under libraries, but from runtime flavor specific location (e.g. src\coreclr\System.Private.CoreLib or src\coreclr\nativeaot\System.Private.CoreLib).
Building libraries tests for NativeAOT can be disabled at project granularity https://github.com/dotnet/runtime/blob/main/src/libraries/tests.proj#L401
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| if (method.HasCustomAttribute("System.Diagnostics", "StackTraceHiddenAttribute") | ||
| || (method.OwningType is MetadataType mdType && mdType.HasCustomAttribute("System.Diagnostics", "StackTraceHiddenAttribute"))) | ||
| || (method.OwningType is MetadataType mdType && mdType.HasCustomAttribute("System.Diagnostics", "StackTraceHiddenAttribute")) | ||
| || (method is Internal.IL.Stubs.ILStubMethod) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why the IL stubs were not a problem before this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #122722 (comment), they did show up as unknowns but this should filter them out.
src/coreclr/vm/debugdebugger.cpp
Outdated
| _ASSERTE(pException != NULL); | ||
| // populate exception with information from the continuation object | ||
| EECodeInfo codeInfo((PCODE)diagnosticIP); | ||
| if (codeInfo.IsValid()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a valid case when the codeInfo is not valid here? If not, it would be nice to add _ASSERTE here.
| #ifndef HAVE_EXKIND_H | ||
| #define HAVE_EXKIND_H | ||
|
|
||
| #include <cstdint> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is seems that since you've moved it to the common.h, it is not needed here anymore, according to @am11's comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that was just regarding <functional> @am11
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@rcj1 I am sorry, the C_FUNC wrapping should be done only for coreclr stuff, not for nativeAOT. I haven't realized we don't do that in the nativeaot asm code, it's been a long time since I worked on those parts. |
This reverts commit 9159f0e.
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This PR implements exception stack trace collection in runtime-async stacks for both coreclr and nativeAOT.
Augmenting stack unwinding logic in AsyncHelpers.CoreCLR.cs to append frames from runtime-async async-await chain to exception stacktrace using DiagnosticIP of Continuation.
Implements ThrowExact helper to throw exception that has been stored in Continuation, while keeping its existing stack trace intact. Accomplished by creating throw helpers that set ExKind.RethrowFlag. Would be open to rename "RethrowFlag" and companions to "NoEraseFlag" or something similar, or leave as is.
ILC changes: Ensuring that stack trace records are emitted for runtime-async methods, and hidden for resumption stubs.
Testing: Added tests for line/method/document correctness including v1-v2 chaining.
@dotnet/ilc-contrib